feat(firmware-upload): add data model, APJ reader and bootloader codec - #2027
Open
iacker wants to merge 3 commits into
Open
feat(firmware-upload): add data model, APJ reader and bootloader codec#2027iacker wants to merge 3 commits into
iacker wants to merge 3 commits into
Conversation
Coverage Report for CI Build 33909389085Coverage at 89.404% (no base build to compare)Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
Collaborator
|
I helped out a bit and did two backend commits. |
amilcarlucas
force-pushed
the
feat/firmware-upload-data-model
branch
from
September 5, 2026 11:27
3b4f967 to
13498f4
Compare
Signed-off-by: Billard <82095453+iacker@users.noreply.github.com>
Move APJ file reading and bootloader protocol handling into a dedicated flight-controller adapter with an injected transport interface. Validate decoded APJ payload sizes exactly, bound decompression, handle malformed board revisions consistently, check padded payload capacity, and support the ArduPilot 33-to-9 board compatibility mapping. Implement revision-2 read-back verification, revision-3+ CRC verification, external-flash erase/program/CRC, serial-open retries, erase/CRC timeouts, safe pre-erase cancellation, and guaranteed transport cleanup. Integrate APJ flashing with the FlightController facade: enter bootloader via MAVLink, require a direct serial connection, release and reopen the serial port, reconnect after flashing, and invalidate cached parameters. Add focused tests for protocol revisions, external flash, retries, cancellation, compatibility, parsing validation, and facade lifecycle.
Bound APJ descriptor and encoded-payload sizes before decoding, and preserve typed error stages throughout the upload lifecycle. Retry bootloader synchronization after entry, discard stale serial input, and use a stable Linux serial-by-path device when available. Require explicit confirmation at the facade boundary and prevent progress callbacks from interrupting flashing. Reconnect using the active connection baud rate rather than the default. Extend the bootloader adapter tests for payload limits, synchronization retries, confirmation, progress stages, verification errors, and reconnect baud preservation. Signed-off-by: Dr.-Ing. Amilcar do Carmo Lucas <amilcar.lucas@iav.de>
amilcarlucas
force-pushed
the
feat/firmware-upload-data-model
branch
from
September 5, 2026 11:27
13498f4 to
9979e66
Compare
Contributor
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Pull request overview
Adds the initial firmware-upload feature foundation (APJ parsing + bootloader protocol codec + upload workflow) to support issue #2017 without introducing UI/serial-MAVLink coupling in the domain model.
Changes:
- Introduces a pure firmware-upload domain model (APJ parsing, compatibility rules, state machine, typed errors).
- Adds a bootloader adapter/client implementing the ArduPilot/PX4 serial bootloader protocol and a FlightController facade entrypoint.
- Adds extensive unit tests (APJ parsing, protocol encoding/decoding, fake bootloader transport, reconnection flow) and an architecture document.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_data_model_firmware_upload.py | New unit tests for APJ parsing, compatibility checks, state machine, and codec behavior with a fake bootloader. |
| tests/test_backend_flightcontroller_bootloader.py | New unit tests for the bootloader backend/client (short reads, rev2 vs rev5, ext flash, retries, facade integration). |
| ardupilot_methodic_configurator/data_model_firmware_upload.py | New domain model: types, parsing, bounds checks, padding, CRC, compatibility policy, and upload state transitions. |
| ardupilot_methodic_configurator/backend_flightcontroller_protocols.py | Adds active_baudrate to the connection protocol for reconnecting after flashing. |
| ardupilot_methodic_configurator/backend_flightcontroller_connection.py | Tracks and exposes active_baudrate across connect/retry flows. |
| ardupilot_methodic_configurator/backend_flightcontroller_bootloader.py | New backend module: APJ file reading limit, bootloader packet codec, BootloaderClient, retrying backend adapter. |
| ardupilot_methodic_configurator/backend_flightcontroller.py | Adds upload_apj_firmware() facade method coordinating bootloader entry, flashing, and reconnection. |
| ARCHITECTURE_firmware_upload.md | New architecture/design doc for firmware upload flow and layering. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+388
to
+397
| if image.metadata.extf_image_size: | ||
| stage = UploadStage.ERASING | ||
| self._report(progress_callback, stage, 0, 1) | ||
| self._erase_external(image.metadata.extf_image_size) | ||
| self._report(progress_callback, stage, 1, 1) | ||
| stage = UploadStage.PROGRAMMING | ||
| chunks = program_chunks(image.extf_image) | ||
| for index, chunk in enumerate(chunks, start=1): | ||
| self._command(encode_extf_prog_multi(chunk)) | ||
| self._report(progress_callback, stage, index, len(chunks)) |
Comment on lines
+11
to
+20
| - `backend_flight_controller_firmware_upload.py` is the I/O adapter. It owns serial | ||
| ports, MAVLink bootloader-entry/reboot commands, the ArduPilot bootloader protocol, | ||
| firmware file reading, and progress events. | ||
| - `data_model_firmware_upload.py` is the business/domain model. It owns firmware | ||
| metadata, board compatibility, validation, workflow state, and user-facing error | ||
| classifications. It does not open files, access serial ports, use Tkinter, or talk | ||
| directly to the flight controller. | ||
| - `frontend_firmware_upload.py` is the GUI. It owns file selection, confirmation, | ||
| progress presentation, cancellation, and translated user messages. It delegates | ||
| validation and upload operations to the model and backend. |
Comment on lines
+376
to
+380
| network_prefixes = ("udp:", "udpin:", "udpout:", "tcp:", "tcpin:", "tcpout:", "ws:", "wss:") | ||
| device = self.comport_device | ||
| if self.master is None or self.comport is None or not device or device.lower().startswith(network_prefixes): | ||
| msg = _("firmware upload requires an active direct serial flight-controller connection") | ||
| raise FirmwareFileError(msg) |
Comment on lines
+219
to
+220
| def program_chunks(image: bytes) -> list[bytes]: | ||
| return [image[offset : offset + PROG_MULTI_MAX] for offset in range(0, len(image), PROG_MULTI_MAX)] |
Comment on lines
+111
to
+112
| for _unused in range(len(self.image), flash_size - 1, 4): | ||
| state = crc32(b"\xff\xff\xff\xff", state) |
| if len(encoded) > MAX_ENCODED_BLOB_SIZE: | ||
| msg = _("APJ {key} exceeds {limit} encoded bytes").format(key=key, limit=MAX_ENCODED_BLOB_SIZE) | ||
| raise FirmwareFileError(msg) | ||
| compressed = base64.b64decode(encoded, validate=True) |
Comment on lines
+255
to
+257
| except (KeyError, TypeError, ValueError, zlib.error) as exc: | ||
| msg = _("APJ {key} is not valid base64+zlib data: {error}").format(key=key, error=exc) | ||
| raise FirmwareFileError(msg) from exc |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
First step of #2017, steps 1 and 2 of the implementation sequence. Adds
data_model_firmware_upload.pywith the APJ reader, the board and flash compatibility rules, the upload state machine, and the bootloader packet codec. Constants and image padding follow ArduPilotTools/scripts/uploader.py. No serial, MAVLink or Tkinter code in this PR.AI assistance was used. I reviewed the changes and ran the tests below.
Checklist
git commit --signoff)Testing
tests/test_data_model_firmware_upload.py, including a full erase, program and CRC verify round trip against a fake rev 5 bootloader